Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

DataTransferObjectError::invalidType : get actual type before mutating $value for the error message #81

Merged
merged 2 commits into from
Jan 8, 2020

Conversation

mfn
Copy link
Contributor

@mfn mfn commented Dec 20, 2019

I believe this is an error and wasn't caught so far because no tests for the actual message did exist.

In \Spatie\DataTransferObject\DataTransferObjectError::invalidType this code was executed:

        $currentType = gettype($value);

but it was done so after $value was possible overwritten like this:

        if ($value === null) {
            $value = 'null';
        }

        if (is_object($value)) {
            $value = get_class($value);
        }

        if (is_array($value)) {
            $value = 'array';
        }

Which basically made gettype($value); to yield string in the majority of cases.

This PR fixes this.
Please also see the 2nd commit stand-alone, showing the change form the old to the new message => 182c925

@@ -79,6 +80,7 @@ public function default_values_are_supported()
public function null_is_allowed_only_if_explicitly_specified()
{
$this->expectException(DataTransferObjectError::class);
$this->expectExceptionMessageRegExp('/Invalid type: expected `class@anonymous[^:]+::foo` to be of type `string`, instead got value `null`, which is NULL/');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR, this message ended in:

instead got value null, which is string

and now

instead got value null, which is NULL

@brendt brendt merged commit 32439f7 into spatie:master Jan 8, 2020
@brendt
Copy link
Contributor

brendt commented Jan 8, 2020

Released as 1.13.2: https://github.com/spatie/data-transfer-object/releases/tag/1.13.2

Thanks for all the PRs!

@mfn mfn deleted the mfn-invalid-type-err branch January 8, 2020 14:49
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants